Skip to content

Comments

Raindex Router Solving Mode#430

Open
rouzwelt wants to merge 8 commits intomasterfrom
2026-01-30-raindex-router-mode
Open

Raindex Router Solving Mode#430
rouzwelt wants to merge 8 commits intomasterfrom
2026-01-30-raindex-router-mode

Conversation

@rouzwelt
Copy link
Collaborator

@rouzwelt rouzwelt commented Jan 30, 2026

Motivation

This PR implements solving for routed Raindex orders through Raindex router arb contract:

  • order A/B -> B/C (sushi) -> order C/A

Solution

Checks

By submitting this for review, I'm confirming I've done the following:

  • made this PR as small as possible
  • unit-tested any new functionality
  • linked any relevant issues or PRs
  • included screenshots (if this involves a front-end change)

Summary by CodeRabbit

  • New Features

    • Added Raindex router as a new trading route and trade type; support for enabling Raindex trades and specifying a v6 Raindex arb address.
    • New counterparty routing, profitability estimation, and simulation flows for Raindex multi-step trades; Pareto-optimal pair selection utility.
  • Tests

    • Extensive unit and integration tests covering Raindex routing, simulation, pricing utilities, YAML parsing, and config validation.
  • Types/State

    • v6 contract set, trade-type enums, and resolver logic extended to include Raindex-specific address handling.

@rouzwelt rouzwelt requested a review from hardyjosh January 30, 2026 20:10
@rouzwelt rouzwelt self-assigned this Jan 30, 2026
@rouzwelt rouzwelt added the new feature New Feature label Jan 30, 2026
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 30, 2026

Walkthrough

Adds Raindex router trade support: new config keys (raindexArb/raindexRouter), v6 RouteLeg ABI and arb4 signature changes, raindex trade mode with profit estimation and simulation (RaindexRouterTradeSimulator), order discovery utilities, validators/YAML parsing updates, tests across modules, and wiring into mode orchestration and simulator unions.

Changes

Cohort / File(s) Summary
Configuration
config.env.yaml, config.example.yaml
Add contracts.v6.raindexArbAddress and orderbookTradeTypes.raindexRouter keys and examples.
Config validation & parsing
src/config/validators.ts, src/config/validators.test.ts, src/config/yaml.ts, src/config/yaml.test.ts
Resolve and validate raindexArbAddress and raindexRouter; include raindexArb in v6 contracts; extensive tests added (including many negative/happy-path cases).
ABI & Types
src/common/abis/orderbook.ts, src/core/types.ts
Add _v6.RouteLeg type, export RouteLeg, update arb4 signature to accept startTakeOrders array, and add TradeType.Raindex.
Mode orchestration & simulator unions
src/core/modes/index.ts, src/core/modes/index.test.ts, src/core/modes/simulator.ts
Import and wire findBestRaindexRouterTrade into enabled trade functions and findBestTrade; extend simulator arg/params unions to include Raindex; tests updated.
Raindex mode implementation
src/core/modes/raindex/index.ts, src/core/modes/raindex/index.test.ts, src/core/modes/raindex/utils.ts, src/core/modes/raindex/utils.test.ts
New Raindex routing module: RouteLegType enum, counterparty discovery, quote collection, estimateProfit, selection/simulation orchestration, and utility functions with tests.
Raindex simulation
src/core/modes/raindex/simulation.ts, src/core/modes/raindex/simulation.test.ts
Add RaindexRouterTradeSimulator with prepare/set-calldata/arb4 encoding, task bytecode handling, prepared params types, and comprehensive tests.
Order management & pairing
src/order/index.ts, src/order/index.test.ts, src/order/pair.ts, src/order/pair.test.ts
Add getCounterpartyOrdersAgainstBaseTokens to discover counterparties against configured bases; export getOptimalSortedList for Pareto-optimal selection; tests added.
State & contracts
src/state/contracts.ts, src/state/contracts.test.ts
Add optional raindexArb to SolverContracts.v6; populate via resolveVersionContracts; handle TradeType.Raindex in versionAddressGetter; tests updated.
Logging / process
src/core/process/log.ts
Include v6 orderbook ABI entry for AfterClear log parsing.
ABI parsing changes
src/common/abis/orderbook.ts
Orderbook V6 structs/signatures now include RouteLeg and parse it where applicable.

Sequence Diagram

sequenceDiagram
    participant Solver as RainSolver
    participant Mode as RaindexMode
    participant Order as OrderManager
    participant Router as SushiRouter
    participant Sim as RaindexSimulator
    participant Chain as Blockchain

    Solver->>Mode: findBestRaindexRouterTrade(orderDetails,...)
    Mode->>Mode: validate v6 orderbook & raindexArb configured
    Mode->>Order: getCounterpartyOrdersAgainstBaseTokens(orderDetails)
    Order-->>Mode: Map<token, Pair[]>
    Mode->>Router: request route/quotes per base token
    Router-->>Mode: route visuals & RP params
    Mode->>Mode: estimateProfit(counterparty, quote)
    Mode->>Mode: select top candidates
    Mode->>Sim: RaindexSimulator.withArgs(candidate)
    Sim->>Sim: prepareTradeParams() (encode route leg, build takeOrders)
    Sim->>Chain: getEnsureBountyTaskBytecode()
    Chain-->>Sim: task bytecode
    Sim->>Sim: getCalldata() → encoded arb4(...)
    Sim-->>Mode: Prepared transaction
    Mode->>Sim: simulate execution
    Sim-->>Mode: SimulationResult
    Mode->>Solver: return best SimulationResult
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Suggested reviewers

  • hardyjosh
🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Raindex Router Solving Mode' directly describes the main feature added: implementing a new solving mode for Raindex router-based arbitrage trades. It clearly summarizes the primary purpose of the changeset.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch 2026-01-30-raindex-router-mode

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@rouzwelt rouzwelt changed the title init Raindex Router Solving Mode Feb 2, 2026
@rouzwelt rouzwelt changed the base branch from 2026-01-22-v6-ob to master February 10, 2026 03:03
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 21

🤖 Fix all issues with AI agents
In `@src/common/abis/orderbook.ts`:
- Line 124: RouteLeg is missing an "as const" assertion so its literal ABI
string widens to string and breaks type inference when used with
parseAbiParameters (e.g., parseAbiParameters(_v6.RouteLeg)). Fix by adding the
as const assertion to the RouteLeg export (the exported constant named RouteLeg)
so it preserves the literal type; ensure any other occurrences in the _v6 ABI
group follow the same pattern to keep parseAbiParameters strongly typed.
- Around line 150-154: The ABI for Arb.arb4 changed to require four args
(orderBook, startTakeOrders[], exchangeData, task); update all call sites that
invoke arb4 (notably in the router and inter simulate functions) to pass the new
exchangeData parameter between startTakeOrders and task, e.g., modify the calls
in simulate.ts where arb4 is invoked to include an appropriate bytes
exchangeData value (or an empty bytes if no data) as the third argument; also
update any unit/integration tests that invoke arb4 to reflect the new
four-argument signature and adjust expected behavior accordingly.

In `@src/core/modes/index.test.ts`:
- Around line 23-29: Remove the leftover commented-out balancer mock in
index.test.ts (the vi.mock call for "./balancer" and findBestBalancerTrade);
either delete those three commented lines entirely or replace them with a brief
inline comment explaining why balancer support is omitted (e.g., removed/handled
in another PR) so future readers aren’t confused—locate the commented block near
the existing vi.mock("./raindex") mock to edit.
- Line 674: The test title string in the it(...) block "should return only
inter-orderbook trade when orderbook is in raindex router set" is misleading;
change that description to accurately reference the raindex router (for example:
"should return only trades routed via raindex router when orderbook is in
raindex router set") so the test name matches the behavior being asserted in the
test case (modify the it(...) title in src/core/modes/index.test.ts).
- Around line 323-328: The test has a copy-paste bug: the raindexResult constant
is created with type: "interOrderbook" instead of the correct "raindex"; update
the object literal in raindexResult (the Result.ok call named raindexResult) to
set type: "raindex" so the test validates raindex results using the correct type
field.

In `@src/core/modes/raindex/index.test.ts`:
- Line 937: The test case name in the it(...) call currently reads "should
return corrrect profit when order max input is lower counterparty max output"
(note the function symbol is the it(...) in
src/core/modes/raindex/index.test.ts) — fix the typo by renaming the test string
to "should return correct profit when order max input is lower counterparty max
output" so the test description is spelled correctly.
- Around line 23-32: The mocked Token class in the vi.mock for "sushi/currency"
returns a plain object from its constructor (return { ...args }), which violates
the noConstructorReturn rule and can break instanceof checks; change the mock to
either use a factory function that returns a proper Token-like object or mutate
the instance instead (e.g., in the Token constructor use Object.assign(this,
args) or populate properties on this) so the constructor returns the instance
rather than a new plain object, keeping the mock defined under
vi.mock("sushi/currency") and preserving the Token symbol.
- Around line 129-133: Replace incorrect assertions that use
.not.toHaveBeenCalledWith() with no arguments — which only checks for a call
with zero args — with .not.toHaveBeenCalled() to assert the mock was never
invoked; update the occurrences that reference
solver.state.contracts.getAddressesForTrade,
solver.orderManager.getCounterpartyOrdersAgainstBaseTokens, and
solver.state.router.sushi!.tryQuote (and the other similar spots in the same
test file) so each uses .not.toHaveBeenCalled() instead of
.not.toHaveBeenCalledWith().

In `@src/core/modes/raindex/index.ts`:
- Around line 28-38: Update the JSDoc comment in src/core/modes/raindex/index.ts
for the RainSolver method that begins "Tries to find the best raindex routed
trade..." and correct the typo "swaped" to "swapped" so the sentence reads
"...swapped through sushi RP"; ensure the change is applied in the JSDoc block
associated with the RainSolver/raindex function signature.
- Around line 196-204: The shared spanAttributes object is being mutated inside
the .map() callback (see Pair.isV4OrderbookV6 and spanAttributes usage) which
can overwrite or leak error context across counterparties; instead create a
local per-counterparty attributes object (e.g., let localSpanAttrs = {
...spanAttributes } or a new object) inside the map/callback and set
localSpanAttrs["error"] when Pair.isV4OrderbookV6(counterpartyOrderDetails)
fails, then return Result.err({... , spanAttributes: localSpanAttrs, ...}) as
SimulationResult so each error carries its own isolated attributes and the outer
spanAttributes remains unchanged for success paths.

In `@src/core/modes/raindex/simulation.ts`:
- Line 19: Update the top doc comment in src/core/modes/raindex/simulation.ts by
replacing the phrase "inter-orderbook trade" with "raindex router trade" so the
comment accurately describes the arguments for simulating a raindex router
trade; locate the comment that currently reads "Arguments for simulating
inter-orderbook trade" and change only the wording to "Arguments for simulating
raindex router trade".
- Around line 59-66: Edit the JSDoc above the RaindexRouterTradeSimulator class
(which extends TradeSimulatorBase) to fix typos and grammar: change "wwith" to
"with", "a external" to "an external", and ensure the phrase reads "Simulates a
trade between two orders with different IO through an external route" (also
correct "two order" → "two orders" if present); update any other small
grammatical issues in that block so the description is clear and consistent.

In `@src/core/modes/raindex/utils.test.ts`:
- Around line 9-141: Tests for calcCounterpartyInputProfit only use
buyTokenDecimals: 18 so scaleTo18 logic is untested; add a new unit test in
src/core/modes/raindex/utils.test.ts that uses a counterparty with
buyTokenDecimals: 6 (or another non-18 value) and a quote where
amountOut/maxOutput are scaled accordingly, then assert both
counterpartyInputProfit and counterpartyMaxOutput reflect the decimal scaling
(reference calcCounterpartyInputProfit and scaleTo18 to craft expected values).

In `@src/core/modes/raindex/utils.ts`:
- Around line 51-58: The function calcCounterpartyInputToEthPrice can divide by
quote.price which may be 0n; add a guard at the start of
calcCounterpartyInputToEthPrice to return 0n when quote.price === 0n (similar to
the check in calcCounterpartyInputProfit), so parsing outputToEthPrice and the
division (outputEthPrice * ONE18) / quote.price only run when quote.price is
non-zero; reference the function name calcCounterpartyInputToEthPrice and the
symbol quote.price when adding the early-return check.
- Line 17: Rename the misspelled variable counterpatryMaxInput to
counterpartyMaxInput everywhere in this file and update all references (the
declaration and uses at the occurrences you noted) so the identifier matches;
search for counterpatryMaxInput and replace with counterpartyMaxInput to avoid
runtime/reference errors and keep naming consistent with related variables like
maxSushiOutput.

In `@src/order/index.test.ts`:
- Around line 22-26: Tests mutate the mocked config object
BASES_TO_CHECK_TRADES_AGAINST created via vi.mock, and vi.clearAllMocks() does
not revert object property changes, so add a reset in beforeEach to restore the
mock state (e.g., call vi.resetModules() or explicitly reassign
BASES_TO_CHECK_TRADES_AGAINST[1] = [] before each test). Update the beforeEach
in src/order/index.test.ts (where vi.clearAllMocks() is used) to either call
vi.resetModules() and re-import the mock or directly reset the
BASES_TO_CHECK_TRADES_AGAINST[1] property so each test starts with a clean
array.

In `@src/order/index.ts`:
- Around line 531-538: The guard checking tokens can throw if
BASES_TO_CHECK_TRADES_AGAINST[this.state.chainConfig.id] is undefined; update
the condition in the loop that references tkn and sellToken so the .every() call
uses a null-coalescing default (e.g.,
BASES_TO_CHECK_TRADES_AGAINST[this.state.chainConfig.id] ?? []) before calling
.every(), ensuring the expression safely falls back to an empty array when the
chain id is not present.

In `@src/order/pair.test.ts`:
- Around line 463-665: The test title for the case validating identical inputs
with differing outputs is misleading; update the it() description string that
currently reads "should handle edge case where all pairs have same input and
output" (the test using createPair hashes hash1/hash2/hash3 and calling
getOptimalSortedList) to something clearer like "should handle edge case where
all pairs have same input but different outputs" so the test name matches the
assertions against getOptimalSortedList.

In `@src/order/pair.ts`:
- Line 82: The JSDoc for getSortedPairList mentions a non-existent parameter
"sortBy"; remove or update that line so the doc matches the function signature:
either delete the `@param sortBy` entry or replace it with descriptions of the
actual parameters accepted by getSortedPairList (use the exact function name
getSortedPairList to locate the comment) and ensure parameter names/types in the
JSDoc match the current signature.
- Around line 166-200: getOptimalSortedList currently mutates the caller's array
and also filters out pairs with maxOutput === 0n due to a truthiness check; fix
by sorting a shallow copy of the input (e.g., use [...options] before calling
.sort) so callers aren't surprised, and change the filter condition in the
pareto selection to explicitly check for undefined/null (e.g.,
opt.takeOrder.quote?.maxOutput !== undefined && opt.takeOrder.quote.maxOutput >=
0n) or at least opt.takeOrder.quote != null to include 0n results if desired;
also add a short inline comment near the filter to explain that 0n outputs are
intentionally excluded or included per the chosen behavior.

In `@src/state/contracts.test.ts`:
- Line 868: Fix the typo in the test description string inside the it(...) block
named "should return undefined for Raindex tradeType when raindexRab is not
available" by changing "raindexRab" to "raindexArb" so the description reads
"should return undefined for Raindex tradeType when raindexArb is not
available"; locate the it(...) in the tests (symbol: the it(...) with that
description) and update the string only—no logic changes.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/core/modes/raindex/simulation.test.ts`:
- Around line 299-312: The compose-error test for setTransactionData is missing
an assertion that getCalldata was not invoked; update the "should return error
if getEnsureBountyTaskBytecode fails with compose error" test to include
expect(getCalldataSpy).not.toHaveBeenCalled() after calling
simulator.setTransactionData(preparedParams). This mirrors the parse-error test
and ensures getCalldata (spy referenced as getCalldataSpy) is not called when
getEnsureBountyTaskBytecode (mocked via getEnsureBountyTaskBytecode) resolves
with Result.err, keeping checks around simulator.setTransactionData and the
EnsureBountyTaskError behavior consistent.
- Around line 244-258: The test "should create takeOrders with correct min/max
float values" currently only checks IOIsInput and data; update the assertions
after calling simulator.prepareTradeParams() to verify takeOrders[0] and
takeOrders[1] include the numeric properties minimumIO, maximumIO, and
maximumIORatio and that they equal the expected values (either the known
constants used to construct the simulator or values available on the simulator
fixture/result); specifically add assertions for
result.value.takeOrders[0].minimumIO, .maximumIO, .maximumIORatio and the same
for takeOrders[1], comparing against the correct expected numbers from the test
setup (or rename the test if you prefer to keep only IOIsInput/data checks).

In `@src/core/modes/raindex/simulation.ts`:
- Around line 78-92: Remove the commented-out destructured bindings from the
this.tradeArgs destructuring so only actual used properties are listed;
specifically edit the destructuring block that defines orderDetails,
counterpartyOrderDetails, maximumInputFixed, blockNumber,
counterpartyInputToEthPrice, counterpartyOutputToEthPrice, quote, rpParams, and
routeVisual and delete the commented items `type`, `solver`, `signer`, and
`profit` to eliminate dead code and keep the destructure clean.
- Around line 136-137: The code accesses
tradeArgs.solver.state.chainConfig.routeProcessors["4"] without guarding for
missing entries, which can make destination undefined and cause
encodeAbiParameters to throw; update the logic where destination is set
(referencing routeProcessors and destination in the route leg construction
inside the function that builds route legs in simulation.ts) to check for a
present routeProcessors["4"] and, if missing, return a SimulationHaltReason (or
similar typed halt) instead of proceeding; ensure you use the same typed halt
used for addresses checks, e.g., create and return a SimulationHaltReason
indicating a missing route processor for chain id "4" so encodeAbiParameters
never receives undefined.
- Around line 159-165: The TakeOrdersConfigTypeV5 construction is using
orderDetails.buyTokenDecimals for IO sizing which is misleading for the
counterparty take order; change the decimals passed to minFloat(...) and
maxFloat(...) to use this.tradeArgs.counterpartyOrderDetails.sellTokenDecimals
(i.e., replace references to this.tradeArgs.orderDetails.buyTokenDecimals for
minimumIO and maximumIO) so the IO bounds reflect the counterparty's sell token
denomination while leaving maximumIORatio, orders, data and IOIsInput unchanged.
- Around line 50-57: The RaindexRouterTradePreparedParams.type defines an
optional price that prepareTradeParams() never sets but trySimulateTrade()
passes to estimateProfit(...), so either set price inside prepareTradeParams()
or stop using it; fix by computing and assigning a bigint price in
prepareTradeParams() (e.g., derive from prepared takeOrders / minimumExpected /
rawtx or from the same pricing logic used elsewhere) and return it on the
RaindexRouterTradePreparedParams object, then keep the trySimulateTrade() call
to this.estimateProfit(prepareParamsResult.value.price) safe; alternatively, if
you cannot compute a meaningful price, remove the price field from
RaindexRouterTradePreparedParams and update trySimulateTrade() to call
estimateProfit with a computed value or skip passing undefined to
estimateProfit. Ensure changes reference RaindexRouterTradePreparedParams,
prepareTradeParams(), trySimulateTrade(), and estimateProfit().

---

Duplicate comments:
In `@src/core/modes/raindex/simulation.ts`:
- Around line 59-66: Fix the typos in the class JSDoc for
RaindexRouterTradeSimulator (which extends TradeSimulatorBase): change "wwith"
to "with" and "a external" to "an external" and ensure the first sentence reads
clearly (e.g., "Simulates a trade between two orders with different IO through
an external route"). Keep the rest of the docblock intact and consistent with
the class purpose and bullet points.
- Line 19: Update the doc comment that currently reads "Arguments for simulating
inter-orderbook trade" to instead read "Arguments for simulating raindex router
trade" so the documentation accurately describes the feature; locate the comment
string "Arguments for simulating inter-orderbook trade" in
src/core/modes/raindex/simulation.ts (immediately above the related
arguments/interface) and replace "inter-orderbook trade" with "raindex router
trade".

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (7)
src/core/modes/raindex/utils.ts (1)

17-24: ⚠️ Potential issue | 🟡 Minor

Division by zero when quote.price === 0n is still unguarded.

Line 23 will throw a RangeError if quote.price is 0n (e.g., no-route or zero-liquidity edge case).

🛡️ Proposed guard
 export function calcCounterpartyInputToEthPrice(
     quote: SushiRouterQuote,
     outputToEthPrice?: string,
 ): bigint {
     if (!outputToEthPrice) return 0n;
+    if (quote.price === 0n) return 0n;
     const outputEthPrice = parseUnits(outputToEthPrice, 18);
     return (outputEthPrice * ONE18) / quote.price;
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/core/modes/raindex/utils.ts` around lines 17 - 24, The function
calcCounterpartyInputToEthPrice can divide by zero when quote.price === 0n; add
an explicit guard after computing outputEthPrice that checks if quote.price ===
0n and return 0n (or another safe fallback) instead of proceeding to the
division, so the expression (outputEthPrice * ONE18) / quote.price is never
executed with a zero denominator.
src/core/modes/raindex/index.test.ts (3)

941-941: ⚠️ Potential issue | 🟡 Minor

Typo "corrrect" in test names — repeated at lines 941, 994, 1045, and 1097 (still unresolved).

-    it("should return corrrect profit with only counterparty input profit", ...
+    it("should return correct profit with only counterparty input profit", ...

Apply the same single-character fix to the three other occurrences.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/core/modes/raindex/index.test.ts` at line 941, Fix the typo in the test
title strings: replace the misspelled "corrrect" with "correct" in all
occurrences of the it(...) test descriptions in index.test.ts (the failing tests
whose descriptions contain "should return corrrect profit with only counterparty
input profit" and the three other duplicated titles). Update each it(...)
description string so it reads "should return correct profit with only
counterparty input profit".

128-138: ⚠️ Potential issue | 🟡 Minor

.not.toHaveBeenCalledWith() (no args) doesn't assert the mock was never called — it only asserts no call with zero arguments (still unresolved).

This pattern appears at lines 130, 133, 134, 179, 180, 233, and 234. Replace all occurrences with .not.toHaveBeenCalled().

✏️ Proposed fix (showing first occurrence block)
-        expect(solver.state.contracts.getAddressesForTrade).not.toHaveBeenCalledWith();
+        expect(solver.state.contracts.getAddressesForTrade).not.toHaveBeenCalled();
         expect(
             solver.orderManager.getCounterpartyOrdersAgainstBaseTokens,
-        ).not.toHaveBeenCalledWith();
-        expect(solver.state.router.sushi!.tryQuote).not.toHaveBeenCalledWith();
+        ).not.toHaveBeenCalled();
+        expect(solver.state.router.sushi!.tryQuote).not.toHaveBeenCalled();
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/core/modes/raindex/index.test.ts` around lines 128 - 138, Several
assertions use .not.toHaveBeenCalledWith() with no args, which doesn't assert
"never called"; replace each such usage with .not.toHaveBeenCalled().
Specifically, update assertions referencing extendObjectWithHeader,
solver.state.contracts.getAddressesForTrade,
solver.orderManager.getCounterpartyOrdersAgainstBaseTokens,
solver.state.router.sushi!.tryQuote, routeProcessor4ParamsSpy,
visualizeRouteSpy, raindexRouterTradeSimulatorWithArgsSpy, and
trySimulateTradeSpy (and any other occurrences in this test file) to use
.not.toHaveBeenCalled() instead of .not.toHaveBeenCalledWith().

24-33: ⚠️ Potential issue | 🟡 Minor

Token mock constructor uses return { ...args }noConstructorReturn lint violation (still unresolved).

Returning a value from a constructor is flagged by Biome's noConstructorReturn rule and breaks instanceof Token checks.

♻️ Proposed fix
         Token: class {
             constructor(args: any) {
-                return { ...args };
+                Object.assign(this, args);
             }
         },
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/core/modes/raindex/index.test.ts` around lines 24 - 33, The Token mock
inside the vi.mock("sushi/currency") should not return an object from its
constructor (which violates noConstructorReturn and breaks instanceof checks);
change the Token class constructor to assign properties onto this (e.g., use
Object.assign(this, args) or set fields directly) and remove the return
statement so instances of Token remain real class instances for instanceof to
work (update the mocked Token constructor in the vi.mock block).
src/core/modes/raindex/index.ts (3)

24-35: ⚠️ Potential issue | 🟡 Minor

JSDoc typo: "swaped" → "swapped" (still unresolved).

- * orders routed through a middle base token swaped through sushi RP
+ * orders routed through a middle base token swapped through sushi RP
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/core/modes/raindex/index.ts` around lines 24 - 35, Fix the JSDoc typo in
the comment block above the RainSolver method: change "swaped" to "swapped" in
the sentence describing middle base token routing through sushi RP; update the
JSDoc near the RainSolver method (the comment that includes "@param this -
RainSolver instance" and the parameters orderDetails, signer, fromToken,
inputToEthPrice, outputToEthPrice, blockNumber) so the text reads "swapped"
instead of "swaped".

185-219: ⚠️ Potential issue | 🟡 Minor

Shared spanAttributes is mutated inside .map() (still unresolved).

When a counterparty fails the v6 check at line 196, spanAttributes["error"] is written to the outer shared object. Multiple failures overwrite each other, and a stale "error" key can survive into the success path returned by the enclosing function.

♻️ Proposed fix — isolate per-counterparty error attributes
-            if (!Pair.isV4OrderbookV6(counterpartyOrderDetails)) {
-                spanAttributes["error"] =
-                    "Cannot trade as raindex router as counterparty order is not deployed on v6 orderbook";
-                return Result.err({
-                    type: TradeType.Raindex,
-                    spanAttributes,
-                    reason: SimulationHaltReason.UndefinedTradeDestinationAddress,
-                }) as SimulationResult;
-            }
+            if (!Pair.isV4OrderbookV6(counterpartyOrderDetails)) {
+                return Result.err({
+                    type: TradeType.Raindex,
+                    spanAttributes: {
+                        error: "Cannot trade as raindex router as counterparty order is not deployed on v6 orderbook",
+                    },
+                    reason: SimulationHaltReason.UndefinedTradeDestinationAddress,
+                }) as SimulationResult;
+            }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/core/modes/raindex/index.ts` around lines 185 - 219, The callback passed
to .map mutates the shared spanAttributes when Pair.isV4OrderbookV6 fails,
causing cross-iteration leakage; fix by creating a per-iteration copy (e.g.,
const localSpanAttributes = { ...spanAttributes }) inside the .map callback
before setting localSpanAttributes["error"], and use that local copy when
constructing the Result.err (or any return that references span attributes);
ensure RaindexRouterTradeSimulator.withArgs and successful returns continue to
use the original spanAttributes without the per-iteration mutation.

265-279: ⚠️ Potential issue | 🟡 Minor

Typo in variable name: orderMaxOuputorderMaxOutput.

The misspelling is used consistently at lines 267, 272, and 279, so there is no runtime bug, but it clashes with the correctly-spelled orderMaxInput nearby and will confuse readers.

✏️ Proposed fix
-    let orderMaxOuput = orderDetails.takeOrder.quote!.maxOutput;
+    let orderMaxOutput = orderDetails.takeOrder.quote!.maxOutput;

     // cap order's io tomax possible trade size from counterparty
     if (counterparty.takeOrder.quote!.maxOutput < orderMaxInput) {
         orderMaxInput = counterparty.takeOrder.quote!.maxOutput;
-        orderMaxOuput =
+        orderMaxOutput =
             orderDetails.takeOrder.quote!.ratio === 0n
                 ? orderDetails.takeOrder.quote!.maxOutput
                 : (orderMaxInput * ONE18) / orderDetails.takeOrder.quote!.ratio;
     }

     // calculate router output and counterparty max input and profit
-    const routerOutput = (orderMaxOuput * quote.price) / ONE18;
+    const routerOutput = (orderMaxOutput * quote.price) / ONE18;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/core/modes/raindex/index.ts` around lines 265 - 279, Rename the
misspelled variable orderMaxOuput to orderMaxOutput to match orderMaxInput and
improve readability; update its declaration and all usages in this block (the
variable assigned after orderMaxInput, the conditional branch where it's set
based on counterparty.takeOrder.quote!.maxOutput and
orderDetails.takeOrder.quote!.ratio, and the subsequent routerOutput calculation
that uses it) so references like routerOutput = (orderMaxOuput * quote.price) /
ONE18 become routerOutput = (orderMaxOutput * quote.price) / ONE18 and any other
occurrences are renamed accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/core/modes/raindex/utils.test.ts`:
- Around line 55-92: Add a unit test in the same suite for
calcCounterpartyInputToEthPrice that covers the edge case where quote.price ===
0n: create a quote like { amountOut: 100n, price: 0n } and call
calcCounterpartyInputToEthPrice(quote, "2.0"), asserting the function returns
0n; this documents the expected behavior and protects against division-by-zero
errors in calcCounterpartyInputToEthPrice.

---

Duplicate comments:
In `@src/core/modes/raindex/index.test.ts`:
- Line 941: Fix the typo in the test title strings: replace the misspelled
"corrrect" with "correct" in all occurrences of the it(...) test descriptions in
index.test.ts (the failing tests whose descriptions contain "should return
corrrect profit with only counterparty input profit" and the three other
duplicated titles). Update each it(...) description string so it reads "should
return correct profit with only counterparty input profit".
- Around line 128-138: Several assertions use .not.toHaveBeenCalledWith() with
no args, which doesn't assert "never called"; replace each such usage with
.not.toHaveBeenCalled(). Specifically, update assertions referencing
extendObjectWithHeader, solver.state.contracts.getAddressesForTrade,
solver.orderManager.getCounterpartyOrdersAgainstBaseTokens,
solver.state.router.sushi!.tryQuote, routeProcessor4ParamsSpy,
visualizeRouteSpy, raindexRouterTradeSimulatorWithArgsSpy, and
trySimulateTradeSpy (and any other occurrences in this test file) to use
.not.toHaveBeenCalled() instead of .not.toHaveBeenCalledWith().
- Around line 24-33: The Token mock inside the vi.mock("sushi/currency") should
not return an object from its constructor (which violates noConstructorReturn
and breaks instanceof checks); change the Token class constructor to assign
properties onto this (e.g., use Object.assign(this, args) or set fields
directly) and remove the return statement so instances of Token remain real
class instances for instanceof to work (update the mocked Token constructor in
the vi.mock block).

In `@src/core/modes/raindex/index.ts`:
- Around line 24-35: Fix the JSDoc typo in the comment block above the
RainSolver method: change "swaped" to "swapped" in the sentence describing
middle base token routing through sushi RP; update the JSDoc near the RainSolver
method (the comment that includes "@param this - RainSolver instance" and the
parameters orderDetails, signer, fromToken, inputToEthPrice, outputToEthPrice,
blockNumber) so the text reads "swapped" instead of "swaped".
- Around line 185-219: The callback passed to .map mutates the shared
spanAttributes when Pair.isV4OrderbookV6 fails, causing cross-iteration leakage;
fix by creating a per-iteration copy (e.g., const localSpanAttributes = {
...spanAttributes }) inside the .map callback before setting
localSpanAttributes["error"], and use that local copy when constructing the
Result.err (or any return that references span attributes); ensure
RaindexRouterTradeSimulator.withArgs and successful returns continue to use the
original spanAttributes without the per-iteration mutation.
- Around line 265-279: Rename the misspelled variable orderMaxOuput to
orderMaxOutput to match orderMaxInput and improve readability; update its
declaration and all usages in this block (the variable assigned after
orderMaxInput, the conditional branch where it's set based on
counterparty.takeOrder.quote!.maxOutput and orderDetails.takeOrder.quote!.ratio,
and the subsequent routerOutput calculation that uses it) so references like
routerOutput = (orderMaxOuput * quote.price) / ONE18 become routerOutput =
(orderMaxOutput * quote.price) / ONE18 and any other occurrences are renamed
accordingly.

In `@src/core/modes/raindex/utils.ts`:
- Around line 17-24: The function calcCounterpartyInputToEthPrice can divide by
zero when quote.price === 0n; add an explicit guard after computing
outputEthPrice that checks if quote.price === 0n and return 0n (or another safe
fallback) instead of proceeding to the division, so the expression
(outputEthPrice * ONE18) / quote.price is never executed with a zero
denominator.

ℹ️ Review info

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 67fbad1 and b26e8ff.

📒 Files selected for processing (4)
  • src/core/modes/raindex/index.test.ts
  • src/core/modes/raindex/index.ts
  • src/core/modes/raindex/utils.test.ts
  • src/core/modes/raindex/utils.ts

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

new feature New Feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant